feat(packaging): ship CommonJS builds alongside ESM for v2 packages#2405
Conversation
Add dual CJS+ESM output to all nine publishable packages via tsdown
format: ['esm', 'cjs'] with fixedExtension, and add a require export
condition (plus main) so require('@modelcontextprotocol/...') works from
CommonJS consumers. Output extensions are normalized across packages
(core moves from .js/.d.ts to .mjs/.d.mts); public import paths are
unchanged.
🦋 Changeset detectedLatest commit: c3241d8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
| --- | ||
| '@modelcontextprotocol/server': patch | ||
| '@modelcontextprotocol/client': patch | ||
| '@modelcontextprotocol/core': patch | ||
| '@modelcontextprotocol/server-legacy': patch | ||
| '@modelcontextprotocol/codemod': patch | ||
| '@modelcontextprotocol/express': patch | ||
| '@modelcontextprotocol/hono': patch | ||
| '@modelcontextprotocol/fastify': patch | ||
| '@modelcontextprotocol/node': patch | ||
| --- |
There was a problem hiding this comment.
🟡 The changeset records all nine packages as patch, but this PR is a feat that adds new backwards-compatible public surface (a require export condition, .cjs/.d.cts artifacts, and a main field on every package), which conventionally warrants a minor bump. Since the packages are in beta pre mode the published version is the same either way, but the generated CHANGELOG will list this feature under "Patch Changes" — consider changing the changeset entries to minor.
Extended reasoning...
What the issue is. .changeset/cjs-support-v2-packages.md marks all nine packages (server, client, core, server-legacy, codemod, express, hono, fastify, node) as patch. The PR itself is titled feat(packaging) and adds new backwards-compatible functionality: every package gains a require condition on each export leaf, new .cjs/.d.cts build artifacts, and a new main field for bare-require fallback. Under semver conventions, a new backwards-compatible feature is a minor bump, not a patch.
Why this is a surface addition, not a fix. The repo's own topology pin test (packages/core-internal/test/packageTopologyPins.test.ts) describes a CJS build as "a new public surface" — i.e. the project already treats dual-format output as an addition to the public API contract, not an internal detail. Repo precedent also points to minor for feature changesets even during the prerelease cycle: the toWebRequest export (#2390), SdkHttpError, and the auth-iss changesets were all recorded as minor, and packages/server/CHANGELOG.md already has "Minor Changes" sections from the 2.0.0-alpha/beta cycle (e.g. #2286, #1689). patch in this repo is used for docs and fixes.
Concrete walk-through of the effect. The workspace is in changesets pre mode (.changeset/pre.json, tag beta), and all nine packages are at 2.0.0-beta.1. When the release PR is generated: (1) changesets reads this file and sees patch for each package; (2) because semver-inc of a prerelease version yields the next prerelease regardless of bump type, the published version is 2.0.0-beta.2 whether the entry says patch or minor — so no version-number harm; (3) however, the changelog generator groups entries by bump type, so the "Ship CommonJS builds alongside ESM…" entry lands under a "Patch Changes" heading in each package's CHANGELOG.md. Consumers scanning the changelog for new capabilities would not expect a new require() entry point to be listed as a patch, and the release notes would misrepresent the nature of the change after the prerelease exits.
Why nothing else catches it. changeset-bot only verifies that a changeset exists and reports the declared bump types (its comment on this PR faithfully lists all nine as "Patch"); nothing validates that the bump type matches the semantic nature of the change.
How to fix. Change each frontmatter entry in .changeset/cjs-support-v2-packages.md from patch to minor (nine one-word edits). No other change is needed; the changeset body text is accurate.
Impact assessment. This is purely a changelog-categorization/convention issue — no build, runtime, or version-resolution behavior is affected, and in pre mode the published version is identical either way. It should not block merge; it's just worth a quick edit so the release notes categorize the CJS support as the feature it is.
There was a problem hiding this comment.
meh doesn't matter imo
packageTopologyPins pinned the packages as ESM-only (no 'require' export condition). This PR intentionally adds CJS, so remove that assertion; the name/export-key/files/bin pins in the file stay. Update the surface-pins doc row to match.
Address review on #2405: - Replace the removed 'ships ESM only' pin with a 'ships dual ESM + CJS' pin in packageTopologyPins.test.ts: asserts type:'module', a .cjs main, and that every export leaf (recursively, incl. _shims runtime conditions) offers both import and require branches — so a future drop of either branch fails CI instead of silently shipping. - Update docs/migration/upgrade-to-v2.md: v2 now ships CJS, so require() resolves natively; drop the obsolete Jest moduleNameMapper workaround and the ESM-only claims. - docs/behavior-surface-pins.md: describe the pinned surface as dual ESM/CJS.
| `moduleNameMapper` workaround to import `@modelcontextprotocol/*`. If you carried a | ||
| v1-era mapping that pinned these packages to their `dist/*.mjs` files, remove it. Vitest |
There was a problem hiding this comment.
🟡 The new Jest section calls the removed moduleNameMapper recipe "a v1-era mapping", but that mapping was this guide's own v2-alpha/beta interim workaround (v1 shipped dist/cjs//dist/esm/ .js builds and never needed one, and the mapped specifiers/dist/*.mjs paths only exist in v2). Consider rewording to "the earlier v2-beta moduleNameMapper workaround" so readers who added it during the beta recognize it and remove the now-obsolete pin.
Extended reasoning...
What the issue is. The rewritten "CommonJS test runners (Jest)" section (docs/migration/upgrade-to-v2.md:258-259) now reads: "If you carried a v1-era mapping that pinned these packages to their dist/*.mjs files, remove it." The mapping being described is unambiguously the moduleNameMapper recipe that this same guide prescribed for the v2 alpha/beta ESM-only packaging — the section this PR deletes explicitly called it "the interim recipe — interim because the packaging shape is still under discussion and a later alpha may make it unnecessary." Users who added it did so while already on v2 prereleases, not on v1.\n\nWhy "v1-era" cannot be right. A v1-era mapping to dist/*.mjs could not have existed:\n\n1. The bare specifiers the recipe maps — @modelcontextprotocol/client, @modelcontextprotocol/server, and the /_shims self-reference — only exist in v2. v1 was the single @modelcontextprotocol/sdk package.\n2. v1 shipped dist/cjs/ and dist/esm/ .js flavor pairs (as this same guide notes a few lines above, ~233-235) and resolved fine under Jest's default CommonJS resolution, so no moduleNameMapper was ever needed for v1, and no dist/index.mjs / dist/shimsNode.mjs paths existed to pin.\n\nStep-by-step reader scenario showing the impact. (1) A user upgraded to 2.0.0-beta.1 a few weeks ago and, following this guide's then-current Jest section, added moduleNameMapper entries pointing @modelcontextprotocol/client at dist/index.mjs and /_shims at dist/shimsNode.mjs. (2) They upgrade to the release that ships this PR and re-read the Jest section. (3) The guide tells them to remove a "v1-era mapping" — they never migrated a Jest config from v1, or they grep for something v1-flavored, don't recognize the mapping they added during the beta as the thing being referred to, and leave it in place. (4) The stale pin keeps routing resolution straight to the .mjs dist files through the transform-based path, silently bypassing the new require condition this PR ships — exactly the workaround the section says is no longer needed.\n\nWhy nothing else catches it. This is prose in a migration guide; no test or lint validates the historical attribution, and the sentence was introduced by this PR's own rewrite (commit c3241d8), so it is in scope for this review.\n\nHow to fix. One-line reword, e.g.: "If you carried the earlier v2-beta moduleNameMapper workaround that pinned these packages to their dist/*.mjs files, remove it" (or "the interim mapping this guide previously prescribed"). Everything else in the rewritten section is accurate.\n\nSeverity. Documentation prose accuracy only — nothing breaks at merge time and the fix is a few words, so this is a nit, not a blocker.
There was a problem hiding this comment.
given v2 is so new, I don't think we need to do within v2 documentation tbh
|
Hey @mattzcarey, I see you've applied the same changes as PR #2033. Feel free to close my PR, and thanks for taking care of it. Edit: btw, looks like both output ends up under the same dir. Imo it would be nicer to split them under |
|
Also see this commit for |
Summary
Restores CommonJS output for the v2 packages, so
require('@modelcontextprotocol/…')works again alongsideimport. The v2 monorepo shipped ESM-only; this brings back the dual CJS/ESM build v1 had, now driven entirely by tsdown.All nine publishable packages are covered:
core,client,server,server-legacy,codemod, and the fourmiddleware/*adapters.What changed
tsdown configs (9) —
format: ['esm']→format: ['esm', 'cjs']plusfixedExtension: true. tsdown emits the second bundle and its interop shims;fixedExtensionnormalizes every package onto the same convention:index.mjs/index.d.mtsindex.cjs/index.d.ctsThis also fixes a pre-existing inconsistency:
corewas emitting.js/.d.ts(it builds withplatform: 'neutral') while every other package emitted.mjs. They now all match.package.jsonexports (9) — each subpath gains arequirecondition besideimport, following the conventional ordering (importfirst,typesfirst within each branch,defaultlast):This covers every export:
client/server.,/stdio,/validators/{ajv,cf-worker}, and/_shims(the runtime conditions stay outer,import/requirenest inside);server-legacy.//sse//auth. Amainfield is added for bare-requirefallback.core's.export is renormalized off.js.codemod'sbinstays ESM; only its library entry goes dual.Changeset — patch bump for all nine packages.
Verification
pnpm -r build— clean (exit 0), 18 dual-format builds, no warnings.require()-loads without error (no ESM-only dependency gettingrequired), andrequire('@modelcontextprotocol/{client,server,node}')resolves through the newrequirecondition by name.scripts/smoke-dist-types.mjstypes check stays clean.main/types/binpaths resolve to emitted files; each subpath is balanced across.mjs/.cjs/.d.mts/.d.cts.Public import paths are unchanged — this is additive.